State<T> + nested/array #[secret] + Fastly dispatch fidelity + app! app-state injection#306
Conversation
…nfig/app-demo test coverage, workspace commands
…dy-mode rename, Internal->500, scaffold CI checks
… roadmap update, macro crate test
Match the edgezero-cli precedent and the workspace-dependency convention. Adds quote to [workspace.dependencies].
…veat - emit `include_bytes!` const in app! output so edits to edgezero.toml trigger a Cargo rebuild without requiring a .rs change - strengthen middleware_sees_introspection_data to assert manifest_json presence and non-empty route list, mirroring dispatch test - add content-type assertion to routes_lists_registered_routes - add security caveat to routing.md Introspection Routes section noting endpoints are unauthenticated and environment.variables values are emitted
… no edgezero.toml change)
… atomic #[action(manifest|routes)] TDD tasks
…lity inject); fix plan single-filter cmd + probe response
…ne; cargo test in Task 10a
…rrying handler struct
Design spec for two upstream edgezero-core/edgezero-macros primitives: - State<T> extractor + RouterBuilder::with_state for app-owned shared state - nested/array #[secret] support via path-qualified SecretField metadata Filed under docs/superpowers/specs/. Includes a maintainer-review appendix (§8) verifying every current-mechanics claim against origin/main @ 42843b1 and folding in the corrections found: http-facade use in the router plumbing, the inaccurate lib.rs re-export step, the omitted validate_excluding_secrets consumer (needs nested-ValidationErrors navigation, not a rename), the per-struct guard-enforcement rewording, and B-3 being forced to the secret_fields() fn lowering.
…sions; model cached app_state (OnceLock) Finding 1: app!(state = <expr>) runs the state expression each time build_router() is called (per request on Fastly), so the demo's app_state() now caches via OnceLock and the guide warns to build heavy state once + hand out Arc clones. Finding 2: handlers.md gains the app!(state = ...) macro path; fastly.md documents run_app_with_request_extensions (JA4/H2/client-IP hook) and owns_logging opt-out.
…tate - handlers.md: app! allows only one state=<expr> (macro rejects duplicates); use an aggregate app-state struct for multiple values. Fix stray '>' typo. - fastly.md: get_tls_ja4() returns Option<&str>; Ja4 wraps String -> .to_owned(). - p0d plan: model the cached OnceLock<Arc<_>> app_state (was fresh Arc per call).
… model - cli-reference.md / cli-walkthrough.md: bundled `edgezero config push` errors (no raw push); typed push writes ONE BlobEnvelope with every field verbatim, including #[secret] key NAMES — nothing is flattened or stripped. Correct the per-adapter mechanics to one (key, envelope_json) entry and fix the stale Fastly command (config-store-entry update --upsert --stdin). - blob-app-config-migration.md: C::SECRET_FIELDS -> C::secret_fields(). - fastly.md: client IP is carried via FastlyRequestContext (only JA4/H2 need the raw hook); fix import path to context::FastlyRequestContext.
- axum.md: the local file is { "<store_id>": "<BlobEnvelope JSON>" }, not a
flat per-leaf map; secret key names are preserved (never stripped/resolved).
Show the AppConfig<C> extractor as the reader; drop the flat store.get example.
- cli-walkthrough.md: Spin secret note + env-overlay example — push preserves
secret key NAMES and writes the nested blob, it does not strip secrets.
- axum config_store.rs: module + method doc comments describe the envelope
entry (outer map still string->string; typed push writes one envelope string).
- spin.md / manifest-store-migration.md: use <your-cli> config push (bundled edgezero config push errors by design; typed push runs from the app CLI). - axum.md + axum config_store.rs: the outer blob key is the *selected* config key — defaults to the logical store id, overridable with --key. - axum config_store.rs: add generated_at to the BlobEnvelope example (required field), matching the fuller shape in the public Axum docs.
… selector - cli-reference.md: add the shipped `config diff` command (typed-only; --format unified/json/structured, --exit-code, --key/--store/--local/--no-env/etc.), and document the config push flags that were missing: --key, --no-diff, --yes/-y. - manifest-store-migration.md: EDGEZERO__STORES__CONFIG__<ID>__KEY is a concrete runtime blob selector (staging/canary), not free-form tuning — dedicated table row + note; disambiguate the free-form row's metavariable to <SUFFIX>. - configuration.md: mention the __KEY selector next to __NAME so operators find it.
dry-run needs a remote read-back to show the diff; Spin Cloud read-back is Unsupported, so the push errors there. Point readers at --local or --yes, matching the runtime error message. (cli-reference.md + blob migration guide.)
…-secrets-spec-review # Conflicts: # crates/edgezero-cli/src/bin/check_no_nested_app_config.rs # crates/edgezero-core/src/handler.rs # crates/edgezero-core/src/router.rs # crates/edgezero-macros/src/app.rs
CI resolves prettier ^3.4.2 to 3.8.3, which normalizes list spacing more strictly than the 3.4.2 I formatted with locally — remove the stray blank line in the Spin push bullet so format-docs passes.
prk-Jr
left a comment
There was a problem hiding this comment.
PR Review
Summary
Four well-separated upstream primitives (State<T>, nested/array #[secret], Fastly dispatch fidelity, app! app-state) with no blocking findings — implementation is solid and the test coverage is exceptional. All findings below are non-blocking suggestions.
😃 Praise
- Test discipline is exceptional: trybuild UI tests with exact stderr, a true end-to-end nested-secret test through
AppConfig<C>::from_requestwith live in-memory stores, an interleaved-poll no-bleed test forState, and Set-Cookie multi-value regression tests in both conversion directions. - The app-demo
app_state()doc comment warning about per-requestbuild_router()on Fastly (theOnceLockpattern) is exactly the operational note downstream users need.
Findings
Non-blocking (body-level)
- ♻️ Three parallel path-walkers: path navigation is implemented three times — the JSON walk (
crates/edgezero-core/src/extractor.rs,resolve_secret_field), the TOML walk (crates/edgezero-cli/src/config.rs,collect_secret_leaves), and the prune walk (crates/edgezero-core/src/app_config.rs,prune_secret_leaf). They are already slightly divergent on optional/array handling (see inline comment on the optional-flag conflation). Full unification across three value types would be over-engineering, but a shared conformance test table (the same path/shape fixtures run against all three walkers) would pin them together cheaply. - 🌱 No CLI test for an optional (
Option<String>) secret leaf:collect_secret_leaves' optional arm has no direct test — the runtime walk covers both null and absent leaves, the CLI covers neither. Oneconfig validatetest with an absent optional leaf closes the gap. - 🌱 Per-request
Vecallocation insecret_fields(): this was aconst; it now allocates per call, andsecret_walkcalls it per request. Negligible next to secret-store I/O today; if it ever shows up in profiles, astd::sync::OnceLock<Vec<SecretField>>in the emitted impl fixes it without an API change. - 📝
TypedSecretEntry.field_name&str→Stringis a public API break (crates/edgezero-adapter/src/registry.rs): fine pre-1.0, and#[non_exhaustive]+ the genericnew()keep call sites compiling. Noting for changelog awareness.
CI Status
- fmt: PASS
- clippy (
--workspace --all-targets --all-features -D warnings): PASS - tests (
--workspace --all-targets): PASS
Rename to remove P0-C/P0-D/P0-CD phase tags from filenames: - specs/2026-07-03-edgezero-p0cd-fastly-dispatch-and-appstate-design.md -> ...-fastly-dispatch-and-appstate-design.md - plans/2026-07-04-edgezero-p0c-fastly-dispatch-fidelity.md -> ...-fastly-dispatch-fidelity.md - plans/2026-07-04-edgezero-p0d-app-macro-state.md -> ...-app-macro-state.md Update the plan-to-plan and plan-to-spec path references accordingly.
…-cycles Address PR review findings (all non-blocking): - secret_walk / collect_secret_leaves: `field.optional` reflects only the LEAF (`Option<String>`); a missing/null intermediate object/array is now a ConfigOutOfDate error in BOTH the runtime and CLI walkers, not a silent skip that degraded to a vaguer serde error. Re-aligns the two walkers. - secret_walk: skip `StoreRef` fields at the loop top (they hold a store id, not a key) — avoids a wasted full-path descent before discarding. - derive: reject DIRECT self-cyclic `#[app_config(nested)]` at compile time (new trybuild UI fixture). Mutual A<->B cycles are undetectable by a proc-macro (one type at a time) — documented. - check_no_nested_app_config footer + configuration.md: opt-in supports `T` / `Vec<T>` only (not Option/Box wrappers); document required-intermediate and cyclic-nesting semantics. - tests: optional-leaf-absent + missing-required-intermediate for both walkers.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Requesting changes for one secret-redaction issue in runtime AppConfig validation. I also left one medium Fastly proxy fidelity comment. CI is green, and the earlier nested-secret review feedback appears addressed, but the secret-value exposure path should be fixed before merge.
… port (P2)
- P1 (security): after secret_walk resolves #[secret] fields to real values,
cfg.validate() runs over the resolved struct. validator error params echo the
rejected value, and that string flowed into EdgeError::ConfigOutOfDate ->
HTTP response/logs, leaking the secret. Redact to the field path only
('app config failed validation for field <path>'). Regression test asserts a
failing resolved secret appears in neither the error message nor the response.
- P2 (fastly proxy): the synthesized Host header was built from uri.host()
only, dropping an explicit target port. Use uri.authority() (host:port) so
http://example.com:8080 sends 'Host: example.com:8080'; backend/TLS SNI still
key off uri.host().
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Approving per request. CI is green and the previously requested redaction and Host-port fixes are present. I left inline suggestions for the remaining follow-up issues I found.
…hildren
- Fastly proxy (userinfo leak): build Host from uri.host()+uri.port(), not
uri.authority() (which includes user:pass@) — no credential leak, port kept.
- Cyclic nesting the derive can't see (mutual A<->B, or path-qualified
Vec<crate::A>): a SecretFieldsRecursionGuard entered in every nested
secret_fields() body panics with a clear message instead of overflowing the
stack. Bare direct self-cycles remain a compile error. (should_panic test.)
- Generic nested children: moved the AppConfigRoot bound check from module
scope into secret_fields() so 'struct Wrapper<T>{ #[app_config(nested)] child: T }'
compiles instead of failing with 'cannot find type T'. (trybuild pass fixture.)
- Docs: cli-walkthrough clarifies typed CLI runs non-secret validators only
(secret-leaf validators run at runtime); configuration.md cycle caveat updated.
Four independent upstream primitives for the trusted-server -> EdgeZero migration. Builds on #300 (introspection routes), now merged; this PR targets
main(merged up to date).Closes #304.
Specs:
docs/superpowers/specs/2026-07-02-edgezero-state-and-nested-secrets-design.md(State<T>+ nested secrets),docs/superpowers/specs/2026-07-03-edgezero-fastly-dispatch-and-appstate-design.md(Fastly dispatch + app-state). Task-by-task plans underdocs/superpowers/plans/(all boxes checked).State<T>extractorState<T>extractor (FromRequestby type from request extensions;Deref/DerefMut/into_inner;500when unregistered).RouterBuilder::with_state<T>stores app state in a singleExtensionsbag; dispatch doesrequest.extensions_mut().extend(state.clone())after the introspection injects (last-write-wins byTypeId). Zero macro change --#[action]composesState<T>via the genericFromRequestpath.docs/guide/handlers.md"Sharing app state".Nested / array
#[secret]SecretField { kind, path: Vec<SecretPathSegment>, optional };AppConfigMeta::secret_fields() -> Vec<SecretField>(wasconst SECRET_FIELDS);dotted_path().#[app_config(nested)]opt-in recurses into sub-struct /Vec<_>fields; acceptsOption<String>; rejectsOption<String>on#[secret(store_ref)]; empty/bare#[app_config]hard-errors;AppConfigRootbound assertion; serderename/rename_allguards extended along the path.secret_walkis a path navigator (nested objects, arrays per-element, optional absent/nullskip,KeyInNamedStoresibling resolved in the innermost parent, dotted[n]errors).validate_excluding_secretspruning.config validate/push/diff;TypedSecretEntrycarries a dotted label.#[app_config(nested)].docs/guide/configuration.md"Nested and array secrets".Fastly
run_appdispatch fidelityset_header->append_headerinfrom_core_responseand the proxy response/request conversion (originSet-Cookieno longer collapses).Hooks::owns_logging()-- platform-neutral opt-out gated in all four adapters'run_app;app!(owns_logging = <bool>)macro argument.run_app_with_request_extensions-- a generic pre-dispatch hook: an app closure reads raw-fastly::Requestsignals (e.g. JA4 / H2 / client IP -- the app supplies the closure and its own type) into a scratchExtensionsbefore conversion, merged into the core request; visible to middleware and theState/extractor layer.app!app-state injectionapp!(state = <expr>)makes the macro-generatedbuild_router()call.with_state(<expr>), reusing theState<T>dispatch injection. Macro-only -- no adapter orHookschange.app-demogains astate = crate::app_state()+State<Arc<DemoState>>handler with an end-to-end test.Tests / CI gates
cargo fmt,cargo clippy --workspace --all-targets --all-features -D warnings,cargo test --workspace --all-targets, feature check (fastly cloudflare spin), spinwasm32-wasip2, the nested-AppConfig audit, app-demo (own workspace) tests + clippy, and the Fastly adapter's wasm32-wasip1 + Viceroy tests (headers + request-extension hook). Green locally and on CI after mergingmainup to date.Notes
cargo test.#[expect]are documenteddead_codeon#[derive(AppConfig)]test fixtures.